Extend none isolation to APIServiceNamespaces#432
Conversation
mjudeikis
changed the title
mjudeikis
force-pushed
the
adjust.none.isolation
branch
from
6554d32 to
ca74cbb
Compare
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt> On-behalf-of: @SAP mangirdas.judeikis@sap.com
mjudeikis
force-pushed
the
adjust.none.isolation
branch
from
ca74cbb to
65d1922
Compare
| default: | ||
| // Default to None isolation strategy if no valid isolation strategy is specified | ||
| logger.V(4).Info("Using default None isolation strategy", "export", export.Name) | ||
| isolationStrategy = isolation.NewNone(r.providerNamespace, providerNamespaceUID) |
There was a problem hiding this comment.
Food for thought, should this be a reconcile error? Is this just hiding broken objects from getting noticed by the admin?
There was a problem hiding this comment.
This is required in the object, so it should never be unset. Will make it logging V2 so its visible in the logs for now,.
|
|
||
| processedSchemas[name] = true // This is only schemas names (suffix) | ||
| isClusterScoped = schema.Spec.Scope == apiextensionsv1.ClusterScoped || schema.Spec.InformerScope == kubebindv1alpha2.ClusterScope | ||
| isClusterScoped = schema.Spec.InformerScope == kubebindv1alpha2.ClusterScope |
There was a problem hiding this comment.
Is this something we should mention in the release-notes? It seems like kube-bind is now doing less automagicalism to determine the correct scope to use, and instead the admin now needs to make sure their InformerScopes are set correctly? Or was the automagicalism already not doing anything? My brain currently can't parse all combinations ^^
There was a problem hiding this comment.
I think this was somehow leaking from old implementation. As much as I understand this didnt change the behaviour.
| // Verify the TLS config was created | ||
| if config == nil { | ||
| t.Fatal("Expected non-nil TLS config") | ||
| return |
There was a problem hiding this comment.
Fatal() already ends the test early, no need for a return.
There was a problem hiding this comment.
yes, but linter does not like it :) something about return early to avoid NPE :D
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
mjudeikis
force-pushed
the
adjust.none.isolation
branch
2 times, most recently
from
2e5a3e9 to
4f0596f
Compare
Signed-off-by: Mangirdas Judeikis <mangirdas@judeikis.lt>
mjudeikis
force-pushed
the
adjust.none.isolation
branch
from
4f0596f to
d02df62
Compare
3 participants
Summary
What Type of PR Is This?
This prs renames
cluster-scope-isolationtoisolationso its broarder termAs
APIServiceNamespaceis key object to map namespaces, make it isolation aware. So now, if you use isolation none, all lands end up in the same namespace with the same names. Meaning we can actually get a twin situation./kind feature
Related Issue(s)
Fixes #
Release Notes